Dialog Component #1214
Dialog Component #1214
Conversation
🦋 Changeset detectedLatest commit: 7ad9af3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
keithamus
force-pushed
the
initial-dialog-implementation
branch
2 times, most recently
from
ef2f621
to
9f3e2bb
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
from
376d524
to
8f0d4aa
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
from
94bfc44
to
20cd643
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
2 times, most recently
from
9489471
to
7f5fd84
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
3 times, most recently
from
7d688af
to
c582828
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
from
a945fa5
to
d574ac6
Compare
keithamus
force-pushed
the
initial-dialog-implementation
branch
from
d574ac6
to
fe6e1c5
Compare
| ModalDialogElement: typeof ModalDialogElement | ||
| } | ||
| interface HTMLElementTagNameMap { | ||
| 'modal-dialog': ModalDialogElement |
There was a problem hiding this comment.
Sidenote: I've been wondering whether we should normalize Primer custom elements with a primer prefix. In this case, we don't use the term "modal" to refer to dialogs except in this custom component due to the hyphen requirement. Another example is the tool-tip element.
Maybe primer-dialog, primer-tooltip, primer-split-page-layout, etc?
Related: ADR on spelling of component names
There was a problem hiding this comment.
We also presumably need to use a different name because we'll be running the Experimental modal-dialog still until we've worked out how we're handling all of the bits of functionality that were added to that version. I like primer-dialog.
keithamus
force-pushed
the
initial-dialog-implementation
branch
3 times, most recently
from
0cb5186
to
9469ea0
Compare
|
👋 I'm not sure familiar with the state of dialog is currently but wanted to bring this audit comment from @jscholes to your attention in case it hasn't been surfaced yet. |
* update height/width to size * fix narrow * remove small portrait * update param descriptions, add hide title
Co-authored-by: Katie Langerman <langermank@github.com>
Co-authored-by: Katie Langerman <langermank@github.com>
Co-authored-by: Katie Langerman <langermank@github.com>
Co-authored-by: Katie Langerman <langermank@github.com>
Co-authored-by: Katie Langerman <langermank@github.com>
Co-authored-by: Katie Langerman <langermank@github.com>
…entation Co-authored-by: Katie Langerman <langermank@github.com>
|
👋 Documenting some tips for rolling this out to dotcom (replacing
The
The For Dialogs wrapped in |
This is the initial implementation of the
Primer::Alpha::Dialogcomponent.This takes some of the code we already have in
github/githubfromPrimer::Experimental::Dialog, however I've dropped some properties for it to avoid pulling in configurations which we may want to later remove - to avoid breaking changes. So for example this doesn't implement helpers around<include-fragment>(those helpers cause unnecessary coupling between the two components, and can be abstracted away into different helpers), nor does this support forms (we need to more carefully consider the interplay between dialogs and forms, especially with regard to how native<dialog>will play a role here).This takes the
modal-dialogweb component in its entirety fromgithub/github. While this is a re-implementation of native<dialog>, and it probably makes sense to simply implement<dialog>instead, unfortunately we've noticed some areas where<dialog>isn't quite suitable: the support is only there in newer browsers, and some browsers have buggy implementations, and we need some time to look over the polyfill to make sure it handles what we want and the trade-offs are worth it. While we haven't had time to assess this yet, the<modal-dialog>component is ready to go, and I believe a later refactor from<modal-dialog>to<dialog>should be possible without any breaking changes.Another area we should look into as a follow-up to this initial implementation is how we can leverage the overlay component (#1143) to simplify some areas of the dialog work.